Codecov Report

Merging #306 into master will increase coverage by 0.23%.
The diff coverage is 68.18%.

@@            Coverage Diff             @@
##           master     #306      +/-   ##
==========================================
+ Coverage      59%   59.24%   +0.23%     
==========================================
  Files          17       17              
  Lines         622      633      +11     
==========================================
+ Hits          367      375       +8     
- Misses        255      258       +3

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf5cac4...f81f3cd. Read the comment docs.

I confess that I think there is some redundancy here... Once we have register_renderable, I imagine we don't really need the macro and register_renderable could also add a show method for WEBIO_NODE_MIME? Why defining one show method is a function and defining two is a macro?

As a counter-proposal, regardless of whether we choose function or macro (I personally prefer a macro not to have a @eval inside a function body), I would propose two methods:

macro register_renderable(T)
    # define show methods for T
end

and

macro register_renderable(f, T)
    # define show methods for T
    # define WebIO.render(x::T) = f(x)
end

To be used with the syntax:

@register_renderable(MyType) do val
    # define corresponding node
end

And we could probably deprecate register_renderable as a function.

Yes, the plan (after I realized that I duplicated some register renderable stuff) is definitely to deprecate the function version.

I also very much like that syntax (in addition to the bare @register_renderable MyType).

@shashi @piever Are y'all happy with this?

There's a bug(?) in Julia that makes the my preferred syntax slightly impossible (see JuliaLang/julia#32301).

# This doesn't work.
WebIO.@register_renderable(MyType) do x
    return node(:p, ...)
end

It does work with the uglier syntax where we put the @ before the WebIO. qualification:

# This works
@WebIO.register_renderable(MyType) do x ... end

It also works if you directly import the name.

# This works
using WebIO: @register_renderable
@register_renderable(MyType) do x ... end)